Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rbd: do not try to run resizefs on an encrypted BlockMode volume #3958

Merged
merged 1 commit into from
Jul 3, 2023

Conversation

nixpanic
Copy link
Member

When a volume has AccessType=Block and is encrypted with LUKS, a resize of the filesystem on the (decrypted) block-device is attempted. This should not be done, as the application that requested the Block volume is the only authoritive reader/writer of the data.

In particular VirtualMachines that use RBD volumes as a disk, usually have a partition table on the disk, instead of only a single filesystem. The resizefs command will not be able to resize the filesystem on the block-device, as it is a partition table.

When resizefs fails during NodeStageVolume, the volume is unstaged and an error is returned.

Resizing an encrypted block-device requires cryptsetup resize so that the LUKS header on the RBD-image is updated with the correct size. But there is no need to call resizefs in this case.

Fixes: #3945

Note: it looks like e2e/rbd.go covers this case already, I am not sure why the issue is not hit in the CI.


Show available bot commands

These commands are normally not required, but in case of issues, leave any of
the following bot commands in an otherwise empty comment in this PR:

  • /retest ci/centos/<job-name>: retest the <job-name> after unrelated
    failure (please report the failure too!)
  • /retest all: run this in case the CentOS CI failed to start/report any test
    progress or results

@nixpanic nixpanic added bug Something isn't working component/rbd Issues related to RBD backport-to-release-v3.9 Label to backport from devel to release-v3.9 branch labels Jun 30, 2023
Madhu-1
Madhu-1 previously approved these changes Jun 30, 2023
Copy link
Collaborator

@Madhu-1 Madhu-1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -512,6 +512,11 @@ func resizeNodeStagePath(ctx context.Context,
if err != nil {
return status.Error(codes.Internal, err.Error())
}

// if this is a AccessType=Block volume, do not attempt filesystem resize
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just add more details that if the filesystem is created by te user it user's responsibility to resize the filesystem.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That should be clear if an application uses BlockMode volumes?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i was looking for why we should not attempt for filesystem resize in this case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, does this extra comment help?

@nixpanic nixpanic requested a review from a team June 30, 2023 07:49
@nixpanic nixpanic requested a review from Madhu-1 June 30, 2023 08:23
@mergify mergify bot dismissed Madhu-1’s stale review June 30, 2023 08:23

Pull request has been modified.

@Madhu-1 Madhu-1 added the ok-to-test Label to trigger E2E tests label Jul 3, 2023
@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.25

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.26

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.25

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.26

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.25

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.26

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-cephfs

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-rbd

@ceph-csi-bot ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Jul 3, 2023
@nixpanic
Copy link
Member Author

nixpanic commented Jul 3, 2023

Issues with the Ceph RPM repository caused downloading/installing updates and dependencies to fail.

@nixpanic nixpanic added the ok-to-test Label to trigger E2E tests label Jul 3, 2023
@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.25

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.26

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.25

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.26

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.25

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.26

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-cephfs

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-rbd

@ceph-csi-bot ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Jul 3, 2023
@nixpanic
Copy link
Member Author

nixpanic commented Jul 3, 2023

@Mergifyio queue

@mergify
Copy link
Contributor

mergify bot commented Jul 3, 2023

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at f60a358

When a volume has AccessType=Block and is encrypted with LUKS, a resize
of the filesystem on the (decrypted) block-device is attempted. This
should not be done, as the application that requested the Block volume
is the only authoritive reader/writer of the data.

In particular VirtualMachines that use RBD volumes as a disk, usually
have a partition table on the disk, instead of only a single filesystem.
The `resizefs` command will not be able to resize the filesystem on the
block-device, as it is a partition table.

When `resizefs` fails during NodeStageVolume, the volume is unstaged and
an error is returned.

Resizing an encrypted block-device requires `cryptsetup resize` so that
the LUKS header on the RBD-image is updated with the correct size. But
there is no need to call `resizefs` in this case.

Fixes: ceph#3945
Signed-off-by: Niels de Vos <ndevos@ibm.com>
@mergify mergify bot added the ok-to-test Label to trigger E2E tests label Jul 3, 2023
@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.25

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.26

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.25

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.26

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.25

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.26

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-cephfs

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-rbd

@ceph-csi-bot ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Jul 3, 2023
@nixpanic nixpanic added the ci/retry/e2e Label to retry e2e retesting on approved PR's label Jul 3, 2023
@mergify mergify bot merged commit f60a358 into ceph:devel Jul 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-release-v3.9 Label to backport from devel to release-v3.9 branch bug Something isn't working ci/retry/e2e Label to retry e2e retesting on approved PR's component/rbd Issues related to RBD
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kubevirt failed to encrypt storage classes using ceph-csi rbd
4 participants